[fix](nereids) stabilize the flaky shuffle_left_join regression test#65769
Open
924060929 wants to merge 1 commit into
Open
[fix](nereids) stabilize the flaky shuffle_left_join regression test#65769924060929 wants to merge 1 commit into
924060929 wants to merge 1 commit into
Conversation
With the nereids distribute planner on, the join in this suite can be planned either as a partitioned shuffle or as a left-to-right bucket shuffle (the left aggregated side shuffled onto the right table's storage buckets), and the two candidates are chosen by cost. That cost depends on two things that are not stable during the test: - the scan row count, which is reported asynchronously after the insert, so an unreported count clamps to 1 and yields a partitioned shuffle, while the real count yields a bucket shuffle; - the bucket-shuffle downgrade gate, which turns bucket shuffle back into a partitioned shuffle when the bucket count is small relative to the instance count, so the plan also depends on the number of backends. As a result the asserted plan flipped between INNER JOIN(PARTITIONED) and INNER JOIN(BUCKET_SHUFFLE) depending on timing and cluster topology. The suite is meant to assert the left-to-right bucket shuffle, which saves one exchange versus a plain partitioned shuffle. Restore that assertion and make it deterministic: - analyze the table with sync so the row count is fixed instead of racing the async tablet report; - set bucket_shuffle_downgrade_ratio=0 so the plan does not depend on the number of backends.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review completed for e33826582d27a50d94187db8af1ab03e96194b80. I found no blocking or correctness issues.
Checkpoint conclusions:
- Goal and proof: The change addresses the stated timing/topology flake. Synchronous full analysis publishes the measured three-row base-index count and
id2NDV before either EXPLAIN, whilebucket_shuffle_downgrade_ratio=0removes the instance-count downgrade. The suite still proves the legacyPARTITIONED/two-exchange plan, now proves the distribute-plannerBUCKET_SHUFFLE/one-exchange plan, and checks identical ordered query results. - Scope and clarity: The patch is focused on one regression suite and adds only the two determinism inputs, the corrected assertion, and explanatory comments.
- Concurrency and lifecycle: No production concurrency or lifecycle logic changes. The existing synchronous analysis task fan-out waits on all tasks and uses concurrent/synchronized publication paths. The regression framework closes the suite JDBC context, so the session-variable override does not leak to other suites.
- Configuration and parallel paths: No configuration item is added. The existing forwarded session variable documents that values
<= 0disable downgrade. The distribute planner exposes both shuffle directions and strictly prefers the smaller one-column aggregate exchange; the legacy planner short-circuits to its existing partitioned path, so setting the ratio before both legs is safe. - Compatibility, persistence, and data correctness: No storage format, symbol, FE-BE protocol, EditLog, transaction implementation, or compatibility behavior changes. The setup insert is committed and synchronized before analysis, and analysis failures propagate instead of silently changing the asserted plan.
- Test quality and results: The plan labels and exchange counts are asserted, both result checks remain ordered, the table is dropped before creation and retained for debugging, and the unchanged generated
.outremains correct. The PR reports three successful runs on a four-backend cluster. Per the review-runner contract, I performed static review and did not run builds or regression tests locally. - Performance and observability: The added full analysis scans only a three-row/two-column test table and adds no production overhead. Existing EXPLAIN, variable, and row logging is sufficient for diagnosing this regression.
- Other review points: No additional negative test, metric, lock, memory-ownership, cloud/shared-nothing, or rolling-upgrade work is applicable to this test-only stabilization.
User focus: no additional focus points were provided; the complete PR was reviewed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
The
nereids_syntax_p0/distribute/shuffle_left_joinregression test is flaky.With the nereids distribute planner enabled, the join in this suite can be
planned either as
INNER JOIN(PARTITIONED)or as a left-to-rightINNER JOIN(BUCKET_SHUFFLE)— the aggregated left side shuffled onto the righttable's storage buckets — and the two candidates are chosen by cost. That cost
is not stable during the test:
unreported count clamps to 1 (which favors a partitioned shuffle), while the
real count favors a bucket shuffle;
isBucketShuffleDownGrade) turns bucketshuffle back into a partitioned shuffle when the bucket count is small
relative to the instance count, so the plan also depends on the number of
backends.
As a result the asserted plan flips between
INNER JOIN(PARTITIONED)andINNER JOIN(BUCKET_SHUFFLE)depending on timing and cluster topology.The suite is meant to assert the left-to-right bucket shuffle, which saves one
exchange versus a plain partitioned shuffle. This PR restores that assertion and
makes it deterministic:
analyze table ... with syncfixes the row count instead of racing the asynctablet report (the count is then read from the collected column stats);
set bucket_shuffle_downgrade_ratio=0removes the dependency on the number ofbackends.
Verified on a 4-backend cluster: the suite now stably produces
INNER JOIN(PARTITIONED)for the legacy-planner leg andINNER JOIN(BUCKET_SHUFFLE)for the distribute-planner leg across parallelism1/4/8 and both before/after stats reporting; the suite was run 3 times, all
green.
Release note
None
Check List (For Committer)